feat(linux, steamvr-launcher): Improve logging for gpu checks, add more checks, update wiki #2648
Conversation
…re logs for igpu. Update wiki for integrated graphics and for vrmonitor paths in all commands.
Meister1593
added
documentation
|
Ideally, after this - user shouldn't need to go to wiki and manually concat commands/envs |
There was a problem hiding this comment.
If we specify the full command everywhere then I think we have to include some section about merging them in case multiple things apply to a user. Tho I'm not sure if providing the full thing doesn't actually make it harder than providing a short guide on how to concatonate things with examples.
Also why are people too stupid to concatonate simple strings???
There was a problem hiding this comment.
less things to to do for user - the better imo
alvr uses wide variety of users, from someone who has no idea what software is, to someone who writes code - and even they can be confused
There was a problem hiding this comment.
giving them full guide how to concatenate multiple strings in specific way just because something has to be aligned like that is a way to make newbie user frustrated already
There was a problem hiding this comment.
Sure, I just want to ensure that we don't cause more issues in case someone needs to apply more than one fix at a time. Because if you just tell them to "paste this" every time they'll either overwrite the other things or end up with multiple %command%s at once.
There was a problem hiding this comment.
Also I accidentally put this on the wrong file, oops, what I said should go for the wiki, not for the checking in code
| warn!("__GLX_VENDOR_LIBRARY_NAME=nvidia __NV_PRIME_RENDER_OFFLOAD=1 __VK_LAYER_NV_optimus=NVIDIA_only \ | ||
| VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/nvidia_icd.json %command%") | ||
| VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/nvidia_icd.json {} %command%", vrmonitor_path_string); |
There was a problem hiding this comment.
I'm pretty sure __VK_LAYER_NV_optimus=NVIDIA_only and VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/nvidia_icd.json do the same thing in two different ways. I'd rather not have it twice if not actually necessary as it ends to rather long copy-pastes.
There was a problem hiding this comment.
You could also extract the whole offload string into a separate variable if you're at it.
There was a problem hiding this comment.
hm... should we leave it like that? i forgot to change this, i mostly left it as is
There was a problem hiding this comment.
VK_ICD_FILENAMES would probably be the better way to fix this, only issue is when on some stupid distro that path doesn't match. And then we can just remove the vulkan layer. But we've already been emitting it like that and I've heard no real complaintd, so the path is probably fine.
There was a problem hiding this comment.
Or rather a long while ago VK_DRIVER_FILES was introduced as a replacement for the icd env var, so we could also use that instead.
|
Either way the wiki things we can always improve later (they're a net improvement, I just think they could be even better) and apart from those two small style things these changes look great. |
|
Sorry for hijacking your pr, I just wanted to try and implement those improvements as quickly as possible if it's still viable to get them into the patch release. |
No worries, it's fine, i just thought it will be in next release then, not in current (so i wasn't exactly in a hurry). |
…re checks, update wiki (#2648) * linux: Add amdvlk/amdgpu-pro check, add vrmonitor path to cmdline, more logs for igpu. Update wiki for integrated graphics and for vrmonitor paths in all commands. * light refactor in passing adapter/vendor tuple * compile fixes * Fix format * Fix format * implement pr suggestions * further improve printed messages --------- Co-authored-by: Leonhard Saam <[email protected]>
Thought that those checks in dashboard are not doing anything good at the moment by printing out only half of the command, so i put vrmonitor fix there, and for all users. Mentioned wiki too.
Added check for amdgpu-pro and amdvlk (since they don't work with steamvr, might as well tell user immediately if it will work)
And updated wiki to make sure all commands have appropriate vrmonitor fix, as many users seem to be confused how to concat them together.